fix: connectivity service#16232
Conversation
deba220 to
c424058
Compare
0261ff4 to
f2a1b3a
Compare
5b5560b to
291868d
Compare
4f091ab to
7b9830e
Compare
7d56e29 to
4d47cb8
Compare
|
@TheCrowned Hello. Thank you for the feedback. I updated the PR. Could you please check via latest APK file once its generated? |
2181611 to
1fb36f6
Compare
…vice.java Co-authored-by: Tom <70907959+ZetaTom@users.noreply.github.com> Signed-off-by: Alper Öztürk <67455295+alperozturk96@users.noreply.github.com>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
71cfd4c to
bfe2f88
Compare
|
@TheCrowned Could you explain your setup I cannot reproduce this issue? Is it local Nextcloud instance? |
|
@alperozturk96 nothing fancy I believe. It works fine on older Nextcloud android versions (and on latest on Linux Mint).
It's remotely hosted on a VPS, publicly accessible hosted on a subdomain.
No VPN. |
Signed-off-by: Alper Öztürk <67455295+alperozturk96@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the app’s connectivity detection and server reachability logic to address incorrect “No internet connection” states (issue #14681), by switching from a broadcast receiver approach to ConnectivityManager.NetworkCallback, deduplicating availability checks, and migrating the connectivity service implementation to Kotlin.
Changes:
- Replaces the Java
ConnectivityServiceImplwith a Kotlin implementation that tracks network state viaNetworkCallbackand notifies listeners. - Introduces keyed caching (
ConnectivityKey) and updates cache invalidation/usage points accordingly. - Updates activities/tests to use the new listener API and adds/updates unit + instrumentation test scaffolding.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/verification-metadata.xml | Adds verification metadata entry for coroutines BOM used by new test dependency. |
| gradle/libs.versions.toml | Adds version + coordinate for kotlinx-coroutines-test. |
| app/build.gradle.kts | Adds kotlinx-coroutines-test to unit test dependencies. |
| app/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.kt | New Kotlin connectivity implementation using NetworkCallback, listener notifications, and server reachability checks. |
| app/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.java | Removes legacy Java implementation. |
| app/src/main/java/com/nextcloud/client/network/ConnectivityService.java | Expands API with listener registration and updates documentation. |
| app/src/main/java/com/nextcloud/client/network/NetworkChangeListener.kt | New listener interface replacing the receiver-based listener. |
| app/src/main/java/com/nextcloud/client/network/ConnectivityKey.kt | Adds a cache key type for per-account/per-baseUrl caching. |
| app/src/main/java/com/nextcloud/client/network/Connectivity.kt | Extends connectivity model with isVPN. |
| app/src/main/java/com/nextcloud/client/network/WalledCheckCache.kt | Reworks caching to be keyed and adds connectivity cache storage. |
| app/src/main/java/com/nextcloud/client/network/NetworkModule.java | Updates DI wiring to construct the service from Context instead of providing ConnectivityManager. |
| app/src/main/java/com/owncloud/android/ui/activity/FileActivity.java | Removes broadcast receiver registration and registers/unregisters as a connectivity listener in lifecycle. |
| app/src/main/java/com/owncloud/android/MainApp.java | Removes broadcast receiver usage and registers as a connectivity listener. |
| app/src/main/java/com/owncloud/android/utils/ReceiversHelper.java | Updates cache clearing to clear keyed connectivity state. |
| app/src/main/java/com/nextcloud/client/di/ComponentsModule.java | Removes injector binding for the deleted NetworkChangeReceiver. |
| app/src/main/java/com/nextcloud/receiver/NetworkChangeReceiver.kt | Deletes the legacy BroadcastReceiver-based implementation. |
| app/src/test/java/com/nextcloud/client/network/ConnectivityServiceTest.kt | Updates unit tests for the new implementation and adds coverage for local-network fallback. |
| app/src/androidTest/java/com/nextcloud/client/network/ConnectivityServiceImplIT.kt | Updates instrumentation test to construct the service with Context. |
| app/src/androidTest/java/com/owncloud/android/UploadIT.java | Updates test connectivity mocks for the new listener API and Connectivity signature. |
| app/src/androidTest/java/com/owncloud/android/files/services/FileUploaderIT.kt | Updates test connectivity mocks for the new listener API. |
| app/src/androidTest/java/com/owncloud/android/AbstractOnServerIT.java | Updates test connectivity mocks for the new listener API. |
| app/src/androidTest/java/com/owncloud/android/AbstractIT.java | Updates test connectivity mocks for the new listener API. |
| app/src/androidTest/java/com/nextcloud/test/ConnectivityServiceOfflineMock.kt | Updates mock to implement the new listener methods. |
| app/src/debug/java/com/nextcloud/test/TestActivity.kt | Updates debug mock to implement the new listener methods. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private fun notifyListeners() { | ||
| scope.launch { | ||
| val available = !isInternetWalled() | ||
| withContext(Dispatchers.Main) { | ||
| listeners.forEach { | ||
| Log_OC.d(TAG, "notifying listeners") | ||
| it.networkAndServerConnectionListener(available) | ||
| } | ||
| } | ||
| } |
| } catch (e: Exception) { | ||
| Log_OC.e(TAG, "exception during server check", e) | ||
| getWalledValueFromException(e) | ||
| } finally { |
| * Checks whether the device currently has an active, validated Internet connection | ||
| * via a recognized transport type. | ||
| * | ||
| * <p>This method queries the Android {@link ConnectivityManager} to determine | ||
| * whether there is an active {@link Network} with Internet capability and an | ||
| * acceptable transport such as Wi-Fi, Cellular, Ethernet, VPN, or Bluetooth.</p> | ||
| * | ||
| * <p>For Android 12 (API 31) and newer, USB network transport is also considered valid.</p> | ||
| * | ||
| * <p>Note: This only confirms that the Android system has validated Internet access, | ||
| * not necessarily that the Nextcloud server itself is reachable.</p> | ||
| * | ||
| * @return {@code true} if the device is connected to the Internet through a supported | ||
| * transport type; {@code false} otherwise. |
| private var connectivityCache = mutableMapOf<ConnectivityKey, Connectivity>() | ||
| private val walledStatusCache = mutableMapOf<ConnectivityKey, Pair<Long, Boolean>>() | ||
|
|
||
| @Synchronized | ||
| fun isExpired(): Boolean = when (val timestamp = cachedEntry?.first) { | ||
| null -> true | ||
|
|
||
| else -> { | ||
| val diff = clock.currentTime - timestamp | ||
| diff >= CACHE_TIME_MS | ||
| } | ||
| fun setValue(key: ConnectivityKey, isWalled: Boolean) { | ||
| walledStatusCache[key] = Pair(clock.currentTime, isWalled) | ||
| } | ||
|
|
||
| @Synchronized | ||
| fun setValue(isWalled: Boolean) { | ||
| this.cachedEntry = Pair(clock.currentTime, isWalled) | ||
| fun clear(key: ConnectivityKey) { | ||
| walledStatusCache.remove(key) | ||
| } | ||
|
|
||
| @Synchronized | ||
| fun getValue(): Boolean? = when (isExpired()) { | ||
| true -> null | ||
| else -> cachedEntry?.second | ||
| fun getValue(key: ConnectivityKey): Boolean? { | ||
| val entry = walledStatusCache[key] ?: return null | ||
| val isExpired = (clock.currentTime - entry.first) >= CACHE_TIME_MS | ||
| return if (isExpired) null else entry.second | ||
| } | ||
|
|
||
| @Synchronized | ||
| fun clear() { | ||
| cachedEntry = null | ||
| fun putConnectivityValue(key: ConnectivityKey, connectivity: Connectivity) { | ||
| connectivityCache[key] = connectivity | ||
| } | ||
|
|
||
| @Synchronized | ||
| fun getConnectivity(key: ConnectivityKey): Connectivity? = connectivityCache[key] |
| whenever(networkInfo.type).thenReturn(ConnectivityManager.TYPE_WIFI) | ||
| whenever(accountManager.getServerVersion(any())).thenReturn(OwnCloudVersion.nextcloud_20) | ||
| connectivityService.updateConnectivity() | ||
| Thread.sleep(200) |
|
blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed. |
|
test-Unit test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/16232-Unit-test-12-34 |
|
APK file: https://github.com/nextcloud/android/actions/runs/26225901991/artifacts/7135975879 |

Fixes: #14681
Deduplicates
isNetworkAndServerAvailablelogicImplements
ConnectivityManager.NetworkCallbackfor better network state listeningRemoves deprecated logics
Adds better documentation
Adds exception handling for http call to prevent temporary issues thus we don't inform user with incorrect availability state
Uses for metered detection
NET_CAPABILITY_NOT_METEREDinstead ofNET_CAPABILITY_NOT_RESTRICTEDRemoves network change receiver uses listener for much better reaction to network change
Converts to Kotlin